Skip to content

Conversion Script Refactoring #35

Open
dprim7 wants to merge 8 commits intoucsd-hep-ex:mainfrom
dprim7:conversion
Open

Conversion Script Refactoring #35
dprim7 wants to merge 8 commits intoucsd-hep-ex:mainfrom
dprim7:conversion

Conversation

@dprim7
Copy link
Copy Markdown
Contributor

@dprim7 dprim7 commented Nov 5, 2024

Refactored convert_full_model.py to be modular
Now runs via CLI with arguments
Tested and bugs fixed
Added hls_conversion_config.yaml (not implemented yet)

@dprim7
Copy link
Copy Markdown
Contributor Author

dprim7 commented Nov 9, 2024

Fixing the formatting to match commit checks has caused 144 changed file instead of 3. I can either revert that last commit that did this and do PR without it, or we can stick to current formatting in CI.
This is making me want to rework the CI on this even more

@dprim7 dprim7 requested a review from Copilot July 10, 2025 01:05
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Refactor of the conversion workflow to be modular, driven via CLI, with environment and job orchestration added.

  • Add Micromamba setup script and Kubernetes job spec for reproducible environments and training runs.
  • Introduce a timing log (time.txt) to record processing duration.
  • Clean up whitespace in loss.py.

Reviewed Changes

Copilot reviewed 89 out of 144 changed files in this pull request and generated 1 comment.

File Description
micromamba_setup.sh Script to create and activate the Conda-like environment
models/quantized-dense-embedding/time.txt Log file capturing working time
loss.py Removed stray blank line
l1metml-job2.yml Kubernetes Job manifest for training
Comments suppressed due to low confidence (2)

models/quantized-dense-embedding/time.txt:1

  • [nitpick] Add a newline or clear delimiter between the seconds and minutes values for readability. For example:
Working Time (s): 19636.02
Working Time (m): 327.27
Working Time (s) : 19636.019178152084Working Time (m) : 327.26698630253475

l1metml-job2.yml:16

  • [nitpick] Consider using YAML's block scalar (|) for this long command to improve readability and maintain consistent indentation, for example:
command: |
  git clone ... && \
  cd L1METML && \
  python train.py ...
	    python train.py --workflowType dataGenerator --input /home/users/dprimosc/data/l1_trigger_ntuples/TTbar  --mode 1 --epochs 500 --maxNPF 100 --batch-size 256 --units 12 36 --output models/quantized-dense-embedding/ --quantized 8 2 --model dense_embedding --compute-edge-feat 0 --model-output models/saved_keras_models/quantized_dense_model_100pf_500epochs_1_test --normFac 1"

Comment thread micromamba_setup.sh
@@ -0,0 +1,2 @@
micromamba create --file environment.yml --name l1metml
Copy link

Copilot AI Jul 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider adding the "-y" flag to the micromamba create command to avoid interactive prompts, e.g., micromamba create -y --file environment.yml --name l1metml.

Suggested change
micromamba create --file environment.yml --name l1metml
micromamba create -y --file environment.yml --name l1metml

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants